Skip to content

fix!: retire the CLI flags llama.cpp's server parser rejects, and guard the contract - #426

Merged
bernardladenthin merged 1 commit into
mainfrom
claude/cli-flag-contract-guard
Sep 9, 2026
Merged

bernardladenthin merged 1 commit into
mainfrom
claude/cli-flag-contract-guard

Conversation

@bernardladenthin

Copy link
Copy Markdown
Owner

Summary

  • Seven flags the Java layer emitted are rejected by llama.cpp's server argument parser, which makes the matching builder methods produce an unloadable model, not a merely ineffective one. LlamaModel.loadModel(parameters.toArray()) hands the ModelParameters map to common_params_parse(..., LLAMA_EXAMPLE_SERVER) as argv, and an unregistered option there is a hard error — arg.cpp throws, common_params_parse returns false, load_model_impl throws LlamaException("Failed to parse model parameters").
  • Two are fresh breakage from the b10878 bump (feat!: upgrade llama.cpp from b10870 to b10878 #425): upstream deleted --mlock and --no-mmap, deprecated at b10092 — the whole deprecation window opened and closed inside eight tags. These have a faithful replacement, so no API is lost: a new args.LoadMode enum + ModelParameters.setLoadMode(LoadMode) expose upstream's -lm/--load-mode, and enableMlock() / disableMmap() keep working, re-pointed to LoadMode.MLOCK / LoadMode.NONE — the exact mapping upstream's own deprecation shim used (LLAMA_LOAD_MODE_MLOCK / LLAMA_LOAD_MODE_NONE), so behaviour is unchanged. Both are now @Deprecated.
  • Five were already dead and are now @Deprecated no-ops that write nothing, keeping existing call sites compiling and loading. ModelFlag.MLOCK / NO_MMAP / DUMP_KV_CACHE are removed from the enum so the broken argv is not reachable through setFlag either — the same reasoning that already excluded FLASH_ATTN.
  • A contract test now enforces this, because no Java test can: ModelFlagTest and ModelParametersExtendedTest assert the string mapping (hasKey("--mlock")), never that llama.cpp still accepts the string, so they stayed green for as long as the flags were dead.

The seven, and why each is dead

Flag Java member Status upstream Now
--mlock enableMlock(), ModelFlag.MLOCK deprecated b10092, deleted b10878 emits --load-mode mlock
--no-mmap disableMmap(), ModelFlag.NO_MMAP deprecated b10092, deleted b10878 emits --load-mode none
--dump-kv-cache enableDumpKvCache(), ModelFlag.DUMP_KV_CACHE removed, no replacement no-op
--hf-repo-v setHfRepoV(String) removed with the OuteTTS-era two-model TTS design no-op
--hf-file-v setHfFileV(String) same no-op
--grp-attn-n setGrpAttnN(int) present in arg.cpp, set_examples({COMPLETION, PASSKEY}) no-op
--grp-attn-w setGrpAttnW(int) present in arg.cpp, set_examples({COMPLETION}) no-op

The last two are the interesting ones: they exist in common/arg.cpp at every tag this project has pinned, so any textual sweep reports them alive. common_params_parser_init's add_opt filters by example at registration time, so they are never registered for LLAMA_EXAMPLE_SERVER — the example this binding parses with — and the parser rejects them exactly like a deleted flag. That is precisely why the guard below drives the real option table instead of grepping upstream sources.

The guard

  • llama/cmake/extract-java-cli-flags.cmake — at configure time, extracts every "--flag" string literal ModelFlag.java + ModelParameters.java can emit into a generated header. Line-oriented so Javadoc mentions ({@code --flash-attn}, prose naming --mlock) are dropped, while enum constants, putScalar/putEnum keys, parameters.put keys and the private ARG_* constants all survive. The Java sources stay the single source of truth, so the two halves cannot drift; it fails loud below a 50-flag floor so a broken extractor cannot make the test vacuously pass.
  • llama/src/test/cpp/test_model_flags.cpp (4 tests) — asserts each extracted flag is present in common_params_parser_init(params, LLAMA_EXAMPLE_SERVER).options. Hermetic: no model, no JVM, no network — common_params_parser_init only fills a struct. It also checks the oracle (the option table must be plausibly large and contain --model) and that the exemption list has not rotted, so neither an empty table nor a stale exemption can hide a real failure. --vocab-only is the one exemption, and deliberately so: it is a project pseudo-flag that jllama.cpp removes via strip_flag_from_argv before common_params_parse sees the argv, to select the vocab-only path.

Falsified before it was trusted. Built against the pre-fix Java sources, the test named exactly the seven flags above — including the two set_examples()-scoped ones my own grep-based audit had missed. After the fix it is green.

Test plan

  • Affected unit / integration tests pass locally
  • CI is green on this branch
  • Docs / CHANGELOG updated where applicable

Run locally on Linux x86_64:

  • ctest 531/531 (was 527; +4 from the new file)
  • mvn test 1759 tests, 0 failures, 0 errors
  • PIT 320/320 mutations killed, 0 NO_COVERAGE, test strength 100% (args.* is a gate target at mutationThreshold 100, so LoadMode needed LoadModeTest — added, mirroring LazyModeTest)
  • spotbugs:check cleansetLoadMode added to the design-intent OCP_OVERLY_CONCRETE_PARAMETER list in llama/spotbugs-exclude.xml, the trap CLAUDE.md documents for any new enum-valued ModelParameters setter
  • spotless:check, clang-format 22.1.8, and javadoc:jar all clean

Not verified locally, and the reason to watch CI here: the CMake extractor uses file(STRINGS) + while(... MATCHES ...) and has only been exercised on Linux with CMake 3.28. macOS, both Windows generators, aarch64, s390x (cross + qemu) and Android are unproven until this PR's C++ Tests matrix runs. That is the main risk in this change.

Also updated: CLAUDE.md (test-file table, total 527 → 531, the SpotBugs rename/addition note), TODO.md (the "five CLI flags" entry closed, corrected to seven), and the b10870–b10878 row in docs/history/llama-cpp-breaking-changes.md, which had explicitly deferred this public-API decision to a follow-up.

Related issues / PRs

Refs #425 (the b10878 bump that deleted --mlock / --no-mmap). Implements the fix prescribed by the "ModelParameters emits five CLI flags the server arg parser rejects" entry in TODO.md, added during the b10649 review (#403).

Checklist

  • I have read CONTRIBUTING.md and CODE_OF_CONDUCT.md
  • My commits follow Conventional Commits
  • No security-sensitive changes

Breaking change

ModelFlag.MLOCK, ModelFlag.NO_MMAP and ModelFlag.DUMP_KV_CACHE are removed (source-breaking for anyone calling setFlag(ModelFlag.MLOCK)), and ModelFlag.values().length drops 34 → 31. Every affected constant guaranteed a failed model load, so there is no working behaviour being taken away. All seven builder methods are retained and still compile.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH


Generated by Claude Code

…rd the contract

`LlamaModel.loadModel(parameters.toArray())` hands the ModelParameters map to
`common_params_parse(..., LLAMA_EXAMPLE_SERVER)` as argv, where an unregistered
option is a hard error rather than a warning. Seven flags the Java layer emitted
are no longer registered, so every caller of the matching builder method got
"Failed to parse model parameters" instead of a loaded model.

Two of them are fresh: b10878 deleted `--mlock` and `--no-mmap` (deprecated at
b10092 -- the whole deprecation window opened and closed inside eight tags).
Those have a faithful replacement, so nothing is lost: a new `args.LoadMode`
enum + `ModelParameters.setLoadMode(LoadMode)` expose upstream's `-lm`/
`--load-mode`, and `enableMlock()`/`disableMmap()` keep working, re-pointed to
`LoadMode.MLOCK` / `LoadMode.NONE` -- the exact mapping upstream's own
deprecation shim used. Both are now `@Deprecated`.

Five were already dead: `--dump-kv-cache`, `--hf-repo-v` and `--hf-file-v` are
gone from llama.cpp entirely; `--grp-attn-n` and `--grp-attn-w` are still in
`arg.cpp` but `set_examples()`-scoped to LLAMA_EXAMPLE_COMPLETION/PASSKEY, so
the server parser rejects them exactly like a deleted flag. Their setters are
now `@Deprecated` no-ops that write nothing, keeping call sites compiling *and*
loading. `ModelFlag.MLOCK`/`NO_MMAP`/`DUMP_KV_CACHE` are removed from the enum
so the broken argv is not reachable through `setFlag` either -- the same
reasoning that already excluded `FLASH_ATTN`.

No Java test could catch any of this: `ModelFlagTest` and
`ModelParametersExtendedTest` assert the string mapping (`hasKey("--mlock")`),
never that llama.cpp still accepts the string, so they stayed green while the
flags were dead. The guard closes that:

  - `cmake/extract-java-cli-flags.cmake` extracts every `"--flag"` literal
    `ModelFlag.java`/`ModelParameters.java` can emit into a generated header at
    configure time, so the Java sources stay the single source of truth.
  - `src/test/cpp/test_model_flags.cpp` asserts each one is registered in the
    real `common_params_parser_init(params, LLAMA_EXAMPLE_SERVER).options`,
    exempting only `--vocab-only` (a project pseudo-flag `strip_flag_from_argv`
    removes before the parse). It also checks the oracle and the exemption list
    themselves, so an empty option table or a stale exemption cannot make it
    pass vacuously.

A grep over `arg.cpp` would not have worked -- it is structurally blind to
example scoping, which is exactly where `--grp-attn-n`/`-w` hid. Run against the
pre-fix sources the test named all seven; after the fix it is green.

Verified locally: ctest 531/531 (was 527), `mvn test` 1759 tests green,
PIT 320/320 killed with 0 NO_COVERAGE, spotbugs clean (`setLoadMode` added to
the design-intent OCP suppression list), spotless, clang-format and
`javadoc:jar` all clean.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH
@bernardladenthin
bernardladenthin merged commit 1b42e9b into main Sep 9, 2026
9 of 15 checks passed
@bernardladenthin
bernardladenthin deleted the claude/cli-flag-contract-guard branch September 9, 2026 19:47
@sonarqubecloud

sonarqubecloud Bot commented Sep 9, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

vaiju1981 pushed a commit to vaiju1981/java-llama.cpp that referenced this pull request Sep 15, 2026
Twelve public builder methods went away, because none of them could reach
llama.cpp. Backward compatibility is deliberately not preserved: a method
that writes a key the receiver discards is worse than no method, since the
call site reads as configuration and behaves as a no-op.

Five wrote request keys that no pinned llama.cpp has ever read --
withTfsZ (tfs_z), withPenalizeNl (penalize_nl), both withPenaltyPrompt
overloads (penalty_prompt) and withUseChatTemplate (use_jinja). The request
schema silently discards unknown fields, so these were invisible at runtime
and uncatchable by any integration test.

Seven wrote CLI flags the server argument parser does not register.
bernardladenthin#426 had already stopped them emitting -- setGrpAttnN, setGrpAttnW,
enableDumpKvCache, setHfRepoV and setHfFileV became no-ops, enableMlock and
disableMmap were re-pointed onto --load-mode -- which kept callers loading but
left the API claiming capabilities it does not have. setLoadMode(LoadMode) is
the whole replacement: LoadMode.MLOCK was --mlock, LoadMode.NONE was --no-mmap.

Verified against the history rather than assumed, because it changes what this
is: all eleven wire names were inherited from kherud/java-llama.cpp at fork
point 49be664 (its own pin was llama.cpp b4916), and SIX of them were already
non-functional at that pin -- the four request keys appear zero times in b4916's
examples/server + common, and --grp-attn-n/-w carried
set_examples({LLAMA_EXAMPLE_MAIN, LLAMA_EXAMPLE_PASSKEY}) there exactly as they
carry {COMPLETION, PASSKEY} at b10883. A seventh, --dump-kv-cache, was alive at
b4916 and already dead at b9994, this repo's first recorded commit. So the
dominant cause is not version drift, it is surface that was never executed
against the receiver; the docs/history writeup lands with the guard in a later
commit of this series.

Two follow-on deletions: ParameterJsonSerializer.buildIntArray had
withPenaltyPrompt(int...) as its only caller and is gone with it, and
ChatAdvancedTest's three model-backed "must produce output" tests for tfs_z and
penalty_prompt are gone -- they passed for years while the server ignored the
parameter, which is precisely the false confidence being removed here. Its
testUseChatTemplateInGenerate is renamed to testMessagesInGenerate and says in
its javadoc that it was always measuring withMessages() alone.

README's InferenceParameters snippets were repaired in passing: besides the two
dead calls they used a set* naming the class has not had for a long time (it has
zero set* methods), so the documented examples did not compile.

Verified: mvn test 1740 tests green across the reactor (19 fewer, matching the
deletions exactly), PIT 320/320 killed at 100%, SpotBugs 0 bug instances,
spotless and javadoc:jar clean.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants